Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Script error in setOffsetOption() if !offsetParent #1184

Closed
wants to merge 1 commit into from
Closed

Script error in setOffsetOption() if !offsetParent #1184

wants to merge 1 commit into from

Conversation

RiZKiT
Copy link
Contributor

@RiZKiT RiZKiT commented Mar 5, 2013

offsetParent.getScroll() had thrown an error if !offsetParent. Because there is already an if condition with return, its moved after that condition.

offsetParent.getScroll() had thrown an error for !offsetParent. Because there is already an if condition with return, its moved after that condition.
@RiZKiT
Copy link
Contributor Author

RiZKiT commented Mar 5, 2013

Oh i see its a duplicate of #1122 but it isn't related to IE only. It happens at least when element is "position: fixed".

@SergioCrisostomo
Copy link
Member

👍

offsetParent = element.measure(function(){
return document.id(this.getOffsetParent());
}),
parentScroll = offsetParent.getScroll();
});
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we use spaces here for indentation so it lines up with the line above? /nitpick

But otherwise I like the solution.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With tabsize = 4 it looks right, I don't know how to fix this.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't use tabs here. Use tabs up to the var indentation and then use spaces from there in. Not a huge deal. If you can't figure it out or don't have time, it's ok.

@anutron
Copy link
Member

anutron commented Apr 24, 2014

🚀

@anutron
Copy link
Member

anutron commented Apr 24, 2014

I just shipped it. 08f564d

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants